Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FEVER for ReAct #39

Merged
merged 72 commits into from
Mar 30, 2024
Merged

FEVER for ReAct #39

merged 72 commits into from
Mar 30, 2024

Conversation

tedasdf
Copy link
Collaborator

@tedasdf tedasdf commented Feb 20, 2024

🤔 Reasoning

🚧 Changes

Mostly copied the orginal ReAct Agent
Change the _prompt_agent by adding a new parameter benchmark_types
Adding new example

Improvement :

  • can make some changes to the output
  • the original version is "Claim" instead of "Question"
  • also test cases

✅ PR Checklist

  • Using this PR template?
  • Linked issue?
  • Added feature?
    • Added/updated docs?
    • Added/updated tests?

@alckasoc alckasoc self-requested a review February 20, 2024 03:41
@alckasoc alckasoc linked an issue Feb 20, 2024 that may be closed by this pull request
@alckasoc alckasoc added enhancement New feature or request good first issue Good for newcomers labels Feb 20, 2024
@alckasoc alckasoc changed the title First version , FEVER for ReAct Feb 20, 2024
Copy link
Member

@alckasoc alckasoc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple design changes and FEVERAgent should be merged into ReActAgent. See comments for more details. Otherwise, good job! 😎

@alckasoc alckasoc self-requested a review February 20, 2024 22:09
@alckasoc alckasoc self-requested a review February 21, 2024 18:41
Copy link
Member

@alckasoc alckasoc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

overall, it looks pretty good! i made a couple suggestions.
also, make sure the unit tests are comprehensive:

  • test all diff benchmark types
  • test all new features added

@alckasoc alckasoc self-requested a review February 22, 2024 01:52
Copy link
Member

@alckasoc alckasoc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks solid so far. I think eventually when we get close to merging this PR, we can remove the alfworld environment and the user can define that. It's important to make sure the ReActAgent has access to the necessary prompts for alfworld. The environment can be user defined. Otherwise, awesome!

@alckasoc alckasoc self-requested a review February 24, 2024 01:38
Copy link
Member

@alckasoc alckasoc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall, it looks pretty good! Just some syntax improvements + some questions + some pytest stuff. Take a look at the comments I left. Also, double check over the code to make sure everything is properly documented, linted, and tested (and, importantly, everything is organized well)!

alckasoc
alckasoc previously approved these changes Mar 29, 2024
@alckasoc
Copy link
Member

Alfworld removed -> library is unstable

  • slow installation
  • had some import errors (no alfworld.agents.environment except after importing alfworld?)
  • env.reset() takes forever (> 10 mins in colab; tested this ~5 times)
  • installation doesn't work on windows (vscode env)

@alckasoc alckasoc merged commit 44ee6bb into main Mar 30, 2024
6 checks passed
@alckasoc alckasoc deleted the sing/generalise_react branch March 30, 2024 01:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature Request]: Generalize ReAct and Reflexion
3 participants